feat(sfs): make resource pool wait timeouts configurable - #1740
Merged
marceljk merged 4 commits intoSep 14, 2026
Merged
Conversation
devpie
force-pushed
the
fix/sfs-resource-pool-timeouts
branch
from
September 4, 2026 13:12
1c1872e to
09296ec
Compare
devpie
force-pushed
the
fix/sfs-resource-pool-timeouts
branch
4 times, most recently
from
September 10, 2026 10:30
6d2a216 to
6644872
Compare
marceljk
reviewed
Sep 10, 2026
marceljk
left a comment
Contributor
There was a problem hiding this comment.
Thanks for your contribution! I left some minor comments
Contributor
Author
|
Follow-up in 7f8825e, after re-reading the result of the previous commit:
I also removed a paragraph from the PR description that still described the state write you asked me to drop. |
marceljk
reviewed
Sep 11, 2026
marceljk
approved these changes
Sep 11, 2026
SerseusWasTaken
approved these changes
Sep 14, 2026
CreateResourcePoolWaitHandler and its update/delete counterparts default to 10 minutes. The resource passed a context without a deadline, so that default was the only limit and no configuration could reach it. A pool that STACKIT needs longer than 10 minutes to provision could not be created at all. The SDK wait handler applies its own timeout only when the incoming context carries no deadline (core/wait.WaitWithContext). Setting a context deadline in each CRUD method therefore replaces the hardcoded value, which is what the new `timeouts` attribute does. Defaults stay at the wait handler value plus core.DefaultTimeoutMargin, so unconfigured resources keep their behavior. The configured timeouts are written to state together with the IDs before the create wait starts. Without that, a failed wait leaves an entry whose refresh and destroy fall back to the default timeouts - on exactly the recovery path those values are needed for. The error raised when the create wait handler gives up now says that Terraform marks the resource tainted and replaces it on the next run, names `untaint` and the import ID, and mentions `timeouts.create` only when this context's deadline is what ended the wait. The handler reports terminal error states and failing polls through the same error, which are not timeouts. TestWaitHandlerTimeoutIsBoundedByContext pins the SDK behavior the attribute depends on, so an SDK bump that enforces the handler timeout unconditionally fails the build instead of silently capping the configured value again.
- shorten the create wait error to the timeout hint, and emit the same hint in update and delete so the three read consistently - drop the write of the timeouts attribute into the partial state - add the timeouts attribute to the resource pool data source as well - replace the create timeout test with the shorter form used for dns, on the existing MockServer - keep a blank line before the timeout blocks in Read and Update
…e review The update wait branch kept the summary "Error creating resource pool" while the appended hint named `timeouts.update`, so the diagnostic contradicted itself. Summary and detail now say "updating", matching what stackitcloud#1741 changes the same line to. The hint starts on its own line again; concatenating it directly onto the wrapped error ran the two sentences together. Two follow-ups in the spirit of the review rather than its letter: the data source read timeout gets the same blank line that was asked for in the resource, and the acceptance-test data source declares a timeouts block, as the dns testdata does for its data source.
The hint that names the configured timeout after a wait ran out is not specific to SFS, so it now lives in stackit/internal/utils as TimeoutHint where other resources and data sources can use it. The move adds a unit test that pins the exact text, including the leading newline, and checks that a cancellation or a wait that failed before the deadline produce no hint.
marceljk
force-pushed
the
fix/sfs-resource-pool-timeouts
branch
from
September 14, 2026 08:21
06224ce to
825422b
Compare
devpie
added a commit
to devpie/terraform-provider-stackit
that referenced
this pull request
Sep 14, 2026
stackitcloud#1740 landed the configurable wait timeouts on the same lines this branch guards. The update wait now reports the merged form: summary and detail from main, including the timeout hint, followed by the nil check from this branch. sfs_test.go keeps the tests of both sides.
8 tasks
devpie
added a commit
to devpie/terraform-provider-stackit
that referenced
this pull request
Sep 14, 2026
Three diagnostics inside Update paths were copied from Create and still reported a creation failure. stackitcloud#1740 fixed the resource pool update wait; this covers the ID guard next to it and both update diagnostics of the share resource. Only the summaries change, the details are accurate as they are.
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
relates to #1737
stackit_sfs_resource_poolcannot create a pool that STACKIT needs more than 10 minutes toprovision.
CreateResourcePoolWaitHandlerand its update/delete counterparts setSetTimeout(10 * time.Minute), and the resource passed a context without a deadline, so thatdefault was the only limit and no configuration could reach it.
This adds a
timeoutsattribute (create/read/update/delete) following the pattern ofdremio/instance.Why a context deadline and not
SetTimeoutcore/wait.WaitWithContextapplies the handler's own timeout only when the incoming contextcarries no deadline:
(core v0.26.0, wait.go)
Setting a context deadline per CRUD method therefore replaces the hardcoded value, and no
SetTimeoutcall is needed. Defaults stay at the wait handler value pluscore.DefaultTimeoutMargin, so unconfigured resources keep their current behaviour.TestSfsResourcePoolCreateTimeoutchecks that a configuredtimeouts.createbounds the create.The wait error
When a wait ends because this context's deadline expired, the error names the configured timeout and
suggests raising it. The wait handler reports terminal error states and failing polls through the
same error, so the hint is added only on the deadline path, and identically in create, update and
delete. The hint is
utils.TimeoutHintinstackit/internal/utils, so other resources can reuse it;TestTimeoutHintpins its text.On point 2 of the issue
The issue also asks to keep the resource in state when the wait times out. That is already
implemented:
utils.SetAndLogStateFieldswrites the IDs before the wait (CONTRIBUTING.md statesthis as project doctrine, and
TestSfsResourcePoolSavesIDsOnErrorcovers it), the frameworkinitialises the create response state to a null object so no unknowns leak, and Terraform keeps the
object and marks it tainted rather than discarding it. What follows is a replace, not a lost entry.
No provider change was needed for that, only the corrected wording of the error.
testdata/resource-pool-max.tfsetstimeouts, and noImportStateVerifyIgnoreis needed for it: terraform-plugin-testing deletestimeoutsandtimeouts.*fromboth sides of the comparison unconditionally, after the ignore loop
(testing_new_import_state.go:398-411, v1.16.0).
The DNS acceptance tests rely on the same behaviour.
Not changed
sfs/sharehas the same hardcoded 10 minutes in all three of its wait handlers. Out of scope forthis issue — happy to follow up if wanted.
Checklist
make fmtexamples/directory) — deliberately not: no example inexamples/has ever carried atimeoutsblock (git log -S timeouts -- examples/is empty), and feat(dns) add timeouts to dns resources and datasources #1345, which addedtimeoutsto the DNS resources, put the demonstrable configuration intestdata/resource-max.tfinstead. Hardcoding durations on a registry page would also pin numbers that go stale when the SDK waiter default moves.make generate-docs(will be checked by CI)testdata/resource-pool-max.tfnow setstimeouts, mirroringdns/testdata/resource-max.tf, soTestAccResourcePoolResourceMaxcovers the attribute across create, import-verify and updatemake test(will be checked by CI)make lint(will be checked by CI)